Skip to content

[AMD][dsv4] Fix 2P1D eval 503s/straggler tail, retune disagg concurrency sweep#2293

Merged
adibarra merged 11 commits into
mainfrom
amd/8k1k-theresa
Jul 22, 2026
Merged

[AMD][dsv4] Fix 2P1D eval 503s/straggler tail, retune disagg concurrency sweep#2293
adibarra merged 11 commits into
mainfrom
amd/8k1k-theresa

Conversation

@ichbinblau

@ichbinblau ichbinblau commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fix a class of DeepSeek-V4-Pro disaggregated-eval failures where the client's retry budget ran out before a transiently-open circuit breaker recovered, producing zero result files:
    • Speed up circuit-breaker recovery (--cb-timeout-duration-secs 15 --retry-max-retries 3) instead of the default 60s open window.
    • Gate benchmarking on an end-to-end router readiness canary (one real generation through the router) instead of just the process-level /readiness check, so the eval never starts against a router whose prefill path isn't actually serving yet.
  • Fix the "8k1k conc-500 straggler tail": decode was admitting requests against the model's (huge) default context length while prefill capped at 9217, so over-length requests were accepted by decode (pre-allocated, waiting on KV) but rejected by prefill (never sends KV) and hung to the client timeout. Decode now derives its admission context length from models.yaml (decode.dp/ep_only/no_dp.context_length), falling back to prefill's value when unset, so both sides reject over-length requests fast and symmetrically.
  • Bump DeepSeek-V4-Pro prefill max_total_tokens from 262144 → 2097152 to match the model's actual KV budget.
  • Retune the dsv4-fp4-mi355x-sglang-disagg non-MTP search space: replace the 2P1D DEP8 sweep (conc 128/256/512/1024 across 2 prefill nodes) with 1P1D DEP8 variants (conc 256/512, 1 prefill node) plus a new 1P1D pure-TP8 (no EP, no DP-attention) scenario sweeping conc [1, 2, 4, 8, 16, 32, 64, 128], and drop the 1P1D DEP8 conc=1024 entry.

Test plan

  • Re-run the previously-failing dsv4-fp4-mi355x-sglang-disagg agentic-eval CI job and confirm the router readiness canary passes and lm-eval produces non-empty result files.
  • Confirm the 8k1k conc-500 sweep no longer shows a straggler tail hanging to client timeout (decode --context-length should now equal prefill's 9217 in the launch logs).
  • Sanity-check the new 1P1D pure-TP8 concurrency sweep ([1, 2, 4, 8, 16, 32, 64, 128]) and the retuned 1P1D DEP8 (256/512) scenarios complete and produce reasonable throughput numbers.

Co-authored-by: @lixiufei-leo, @inkcherry, @Duyi-Wang

ichbinblau and others added 6 commits July 20, 2026 03:14
- Speed circuit-breaker recovery and gate benchmarking on an end-to-end
  router readiness canary, so a transient trip or a not-yet-serving
  router doesn't 503 every request until the client's retry budget is
  exhausted (port of PR #2255).
- Keep decode's admission context-length symmetric with prefill
  (falling back to prefill's value when unset in models.yaml), so
  over-length requests are rejected fast by both sides instead of
  hanging on decode while waiting for a KV transfer prefill never
  sends (port of PR #2257).

Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: inkcherry <mingzhi.liu@amd.com>
Signed-off-by: Theresa Shan <theresa.shan@amd.com>
Signed-off-by: Theresa Shan <theresa.shan@amd.com>

Co-authored-by: lixiufei-leo <Xiufei.Li@amd.com>
Signed-off-by: Theresa Shan <theresa.shan@amd.com>
…MTP search space

Drop the 2P1D DEP8 (conc 256/512/1024) and 1P1D DEP8 conc=1024 entries
and add a 1P1D pure-TP8 (no EP, no DP-attention) scenario sweeping
conc [1, 2, 4, 8, 16, 32, 64, 128] for dsv4-fp4-mi355x-sglang-disagg.

Co-authored-by: Cursor <cursoragent@cursor.com>
Squash of f7295a5 "remove decode context_length", 0be1d30
"Revert 'remove decode context_length' for DeepSeek-V4-Pro", and
bfb8988 "remove decode context_length ending with a newline" into
a single commit with the same net result.

Signed-off-by: Theresa Shan <theresa.shan@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ichbinblau
ichbinblau requested a review from a team July 21, 2026 08:53
@ichbinblau ichbinblau added AMD full-sweep-enabled full-sweep-fail-fast-no-canary Full sweep, no canary gate; first failure in a matrix cancels that matrix and removed full-sweep-fail-fast-no-canary Full sweep, no canary gate; first failure in a matrix cancels that matrix labels Jul 21, 2026
Comment thread configs/amd-master.yaml
Comment thread benchmarks/multi_node/amd_utils/server_sglang.sh Outdated
ichbinblau and others added 3 commits July 21, 2026 09:07
Per the automated PR reviewer: any edit to configs/amd-master.yaml
requires a paired perf-changelog.yaml entry. Documents the 503/
straggler-tail fixes, the max_total_tokens bump, and the 2P1D DEP8 ->
1P1D DEP8/pure-TP8 concurrency sweep retune from PR #2293.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve conflicts in perf-changelog.yaml and server_sglang.sh. Keep main's
agentic router settings while preserving this branch's DI circuit-breaker
recovery tuning and readiness canary via IS_AGENTIC.

Co-authored-by: Cursor <cursoragent@cursor.com>
The canary poll loop was a plain while/curl loop, unlike every other
blocking wait on node rank 0 (container barrier, servers-up barrier,
/readiness health barrier), so a prefill crash right after /readiness
passed just looked like repeated 503s and burned the full
ROUTER_CANARY_TIMEOUT (default 600s) instead of failing fast. Extract
the loop into run_router_canary() and run it under wait_or_die so a
dead prefill pid aborts it in ~5s like the surrounding barriers.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

@ichbinblau

Copy link
Copy Markdown
Collaborator Author

/reuse-sweep-run 29821778206

@github-actions

Copy link
Copy Markdown
Contributor

@chunfangamd

Copy link
Copy Markdown
Collaborator

@github-actions

Copy link
Copy Markdown
Contributor

@chunfangamd chunfangamd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a PR reviewer and CODEOWNER, I have reviewed this and have:

  • Verified that as of the moment of typing this, this is the latest version of PR_REVIEW_CHECKLIST.md
  • Verified that the general code quality meets the InferenceX standard and does not make the code quality any worse.
  • Verified that this PR has passed PR validation. Please link to GitHub Action workflow that shows this. Link: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29821778206
  • Verified that this PR passes evals. Please link to GitHub Action workflow that shows this. Range: 96.13%-96.36%; link: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29821778206
  • Verified that speculative decoding PRs uses chat templates to align the AL distribution to real world
  • For agentic workloads: verified that speculative-decoding configs (EAGLE / MTP / draft models) run with simulated synthetic acceptance, with the acceptance-length value taken from the committed golden AL curve in golden_al_distribution/ for that model, thinking mode, and draft length. A submission may choose any supported draft length, but it may not substitute a different acceptance target.
  • Verified that the model architecture isn't changed with benchmark hacks like using --hf-overrides to skipping indexer for every x layers on models that don't natively support this. As a general rule, we won't accept optimizations that reduces the number of model architecture FLOPs. Anything that makes that same computation run faster is fair game; FLOPs at lower precisions is fine, given that the config passes private evals. As an general north star princple, we should only use optimizations which is used in production by customers that care about accuracy
  • If an company claims that they support vLLM/SGLang as first class LLM inference engines on their hardware, I have verified that the respective vLLM submission made using upstream https://hub.docker.com/u/vllm docker repo, upstream SGLang https://hub.docker.com/u/lmsysorg docker repo. The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet as supported by vLLM/SGLang community maintainers
  • If an company claims that they support vLLM/SGLang as first class upstream in-tree LLM inference engines on their hardware, I have have verified that the respective vLLM/SGLang submission has been made before additional frameworks (TRT-LLM, ATOM, etc.). The only exceptions are for new hardware, such as MI455X UALoE72, Vera Rubin NVL72, Rubin NVL8, etc., and for new model architectures where there is an actual reason why vLLM/SGLang does not fundamentally support them yet.
  • Verified that every single-node vLLM/SGLang recipe in this PR is documented in the official vLLM recipes and/or the SGLang cookbook:
    • I linked the corresponding upstream PR in the vLLM recipe repo or SGLang repo and verified that it is MERGED before this InferenceX PR merges. An opened, draft, or closed-without-merge upstream PR does not satisfy this requirement. If the matching recipe was already published, I linked the published recipe/cookbook page in the additional detail section below. No need to update the recipe
  • Verified that this PR does not patch the inference engine or serving stack — the pinned image must run as shipped. This covers .patch files / git apply / patch, inline patches embedded in benchmark scripts (e.g. a python3/sed heredoc that rewrites installed engine sources before serving), in-place edits of site-packages, monkey-patching, overwriting container files, and installing forked/rebuilt engine wheels on top of the pinned image. The only exception is a patch covered by a filled-out waiver at docs/waiver/<PR_NUMBER>.md — named after the PR that introduces the patch and filed in that same PR, stating what is patched, why the unmodified upstream image cannot run this benchmark, the upstream PR/issue link, and the removal plan — which I have linked below in the additional detail section.
  • If any of the above criteria cannot reasonably be satisfied, I have provided additional reasoning below.

Additional detail section:

  • insert any additional info here
    No needs to change the recipe

Signed: @chunfangamd

@Klaud-Cold

Copy link
Copy Markdown
Collaborator

✅✅✅ Verdict: PASS ✅✅✅

✅ Check 0 (CODEOWNER): PASS — chunfangamd is a listed owner of configs/amd-master.yaml; the other changed paths carry only the catch-all.
✅ Check 1 (passing sweep on in-PR commit): PASS — commit 208bff46 (currently in this PR) has all executed benchmark jobs green: multi-node 8k1k / ×3 and multi-node eval / ×2 all success in run 29821778206; single-node lanes skipped by design on this multi-node-only PR.
✅ Check 2 (evals real): PASS — that run's eval_results_all artifact shows GSM8K em_strict 96.13% (1P1D TP8 c128) and 96.36% (1P1D DEP8 c512), n_eff=1319 each, matching the sign-off's stated range; run image lmsysorg/sglang-rocm:v0.5.14-rocm720-mi35x-20260701 matches this PR's config entry.
➖ Check 3 (recipe linked & merged): N/A — disaggregated/multi-node submission (benchmarks/multi_node/**, multinode: true / disagg: true, framework: sglang-disagg); the recipe-link requirement applies to single-node recipes only, so the sign-off's "no recipe update required" note is legitimate.
✅ Check 4 (reuse command): PASS — /reuse-sweep-run 29821778206 posted by ichbinblau (COLLABORATOR).
✅ Check 5 (latest checklist): PASS — every current-template item is present and checked; the one unchecked item is the agentic spec-decode conditional, which does not apply to this PR (no speculative-decoding changes).
✅ Check 6 (upstream image & engine-first): PASS — image is from the upstream SGLang org (lmsysorg/sglang-rocm:...) on MI355X; no non-vLLM/SGLang framework entry added.
✅ Check 7 (no architecture hacks): PASS — no --hf-overrides/model-config edits; changes are decode admission --context-length, KV max_total_tokens, router circuit-breaker tuning, and a readiness canary — none reduce model FLOPs.
➖ Check 8 (spec-decode chat template): N/A — no speculative-decoding changes (all touched search-space entries are spec-decoding: "none", DECODE_MTP_SIZE=0).
✅ Check 9 (no engine patches): PASS — launch-orchestration changes only; no engine-source rewriting, monkey-patching, container-file overwrites, or engine wheel installs.
➖ Check 10 (agentic golden AL): N/A — no agentic speculative-decoding changes in this PR.

@HaiShaw

HaiShaw commented Jul 22, 2026

Copy link
Copy Markdown

Can we get help to merge this? Thanks!
cc @functionstackx , @1am9trash , @billishyahao

@functionstackx

Copy link
Copy Markdown
Collaborator

@adibarra

@adibarra
adibarra merged commit 89696c5 into main Jul 22, 2026
30 checks passed
@adibarra
adibarra deleted the amd/8k1k-theresa branch July 22, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

6 participants